Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Move and delete models using interactive markers #28

Merged
merged 11 commits into from
Oct 15, 2017

Conversation

robustify
Copy link
Contributor

To try it out:

  • Launch the default flatland world.
    roslaunch flatland_server server.launch
  • Everything in the Rviz viewer should look like it does normally on startup.
  • After going into "interact" mode with the Rviz toolbar, the interactive marker display will activate.
  • Leaving interact mode will deactivate the interactive marker display.
  • Drag the models around using either the green cubes or the red body markers.
  • Let go to move the model to that new pose.
  • Right-click the green cubes or the red body markers and click "Delete Model" to... you know

The tween model 'walker1' ignores any attempts to move it, and I haven't dug into it to see why. Perhaps someone can look into that, or we can detect the type of plugin and not generate interactive markers for tweens.

Also, I couldn't think of a good way to spawn models using interactive markers. There could be a fixed marker that has a context menu to spawn a model, but setting the parameters normally specified in the world YAML file seems to be a challenge. Maybe a dynamic reconfigure GUI instead and use a bool parameter as a trigger button? This is definitely something to discuss.

@josephduchesne
Copy link
Member

josephduchesne commented Oct 13, 2017

This looks cool. I've got to get to bed but I'll try it out first thing tomorrow.

I'll have to look into the tween interaction. The tween plugin is fairly "dumb" about motion (it just teleports things around between start and end). It could be made relative to the current position which would probably cause it to interact nicely with this. I don't think it needs to be fixed to merge this in.

We actually have a Spawn Model rviz tool plugin already. It has one minor issue (#15) but otherwise works fairly well. Hit the (+) button in Rviz and add it to your toolbar if you haven't tried it.

See it in use here (from the docs):
image of spawn model plugin

@josephduchesne
Copy link
Member

It occurs to me that now that we can add, move and delete models from the world, we'll probably also want to implement a "save world" and/or "save world as" service call soon. I'll add that to the trello

@robustify
Copy link
Contributor Author

Ah yes, I checked out the model spawn tool earlier, haha. I agree; saving worlds for later would be great.

@josephduchesne
Copy link
Member

I think if the tween plug in stores the old position at the end of its update loop, then gets its delta at the beginning of the update loop ("how much was I moved") then applied it to its start/end position it would handle being moved. It may just make a giant mess though.

@josephduchesne
Copy link
Member

I had a chance to try this branch out. I like the feature, but I'm not sure if one handle for translate+rotate is the best solution. Two notes:

  • There is odd behaviour if you grab the "back" of the interactive marker towards one side. The model is dragged but at an angle.
  • It might make more sense to have a pair of markers, one at the center that only translates the model on drag, and another offset from the model that allows only rotation. This would probably allow for more precise positioning (but maybe less immediate interaction depending on what you plan to use this for). Thoughts?

Fixed minor whitespace lint error
@josephduchesne
Copy link
Member

josephduchesne commented Oct 13, 2017

Looking at the interactive markers available, maybe marker similar to this one with rotate on Z axis and translate on X/Y axis?
http://wiki.ros.org/rviz/Tutorials/Interactive%20Markers%3A%20Basic%20Controls#Quadrocopter

@mbrousse
Copy link
Collaborator

mbrousse commented Oct 14, 2017 via email

@robustify
Copy link
Contributor Author

I made the changes to implement the two separate controls for you to try out.

I agree that two separate controls for translation and rotation would provide more precise positioning. However, the benefit of having a single control is for when the robot is moving when you want to reposition it. If you want to change both its position and orientation, you'll have to catch the robot's rotation control after it starts moving when you release its position control, or vice versa.

There are two ways I can think of to get the benefits of the two separate controls without the annoyance of having to grab the second control after the robot starts moving again:

  • Add a "pause simulation" toggle button to the Rviz toolbar like the "spawn model" button. This could be a useful feature anyway.
  • Add the original move/rotate control to the body line strip marker. This way, you could translate the model with the green cube, rotate it with the ring, or do both at once using the red body line strip.

I also realized that letting the simulation continue while the user is dragging an interactive marker could be undesirable in some situations. I made some minor modifications to this branch to skip the physics and plugin updates while the interactive marker server is between mouse down and mouse up events, and it seems to work nicely.

However, since this is a little off topic for this PR, I'll make another one later once we've agreed on the interactive marker behavior itself, and you think it's a good idea that is worthy of discussion and integration.

@josephduchesne
Copy link
Member

josephduchesne commented Oct 15, 2017

Thanks for the excellent PR. I agree that splitting the functionality makes it harder to interact with moving models.

I think both proposed options are good but the pause/resume button would be more generally useful (I can think of several other use-cases for that). If we do add a play/pause feature it could be generalized to a service to handle the rviz/flatland_viz integration. That should allow for pausing, changing one or more models, then resuming.

Also, thank you for keeping the scope of this PR focused. That's a good skill to have!

On an entirely different note, you should sign up for https://hacktoberfest.digitalocean.com/ to potentially get a free T-shirt for open source contributions this month. I think you're at 2/4 or 3/4 pull requests depending on if the declined one counts, and half way through the month. If you happen to submit another 1-2 PRs to this or any other github repo you should be eligible. (If you're into free t-shirts that is!)

I'll try to update the tween plugin to play more nicely with interactive markers.

@josephduchesne josephduchesne merged commit a3e7c48 into avidbots:master Oct 15, 2017
@robustify robustify deleted the interactive-markers branch October 15, 2017 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants